-
Notifications
You must be signed in to change notification settings - Fork 189
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Jacoco XML report #86
Conversation
Neat! Thanks for this. I'll try to look at it this weekend |
diff_cover/tool.py
Outdated
elif jacoco_xml_roots: | ||
coverage = JacocoXmlCoverageReporter(jacoco_xml_roots, src_roots) | ||
else: | ||
assert False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats happening here? Should we be raising something specific?
for root in self._src_roots: | ||
_files = [_file | ||
for _file in pkg.findall('sourcefile') | ||
if GitPathTool.relative_path(os.path.join(root, pkg.get('name'), _file.get('name'))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick, but perhaps making this a named method can make it clearer what this is doing.
maybe something like _get_measured_source_path(src_root, package_name, file_name)
# First case, need to define violations initially | ||
if violations is None: | ||
violations = set( | ||
Violation(int(line.get('nr')), None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any idea what 'nr' stands for? Can it be a constant?
This is looking pretty good. Thank you! I left a couple small comments. |
Thanks! Comments addressed. Let me know if you need more changes. Please see #87 - I don't think this should be a separate release. |
Implement support for jacoco code coverage (xml output, not the exec format)
XML Report DTD can be found here: https://www.jacoco.org/jacoco/trunk/doc/
Addresses issues #80 and #82